Skip to content

Added link in AesManaged to article warning of timing vulnerabilities… #2609

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Mar 2, 2020
Merged

Added link in AesManaged to article warning of timing vulnerabilities… #2609

merged 2 commits into from
Mar 2, 2020

Conversation

samrueby
Copy link
Contributor

… with CBC-mode symmetric decryption using padding.

Summary

Added a link to an important article from Microsoft about the latest recommendations for securely handling encrypted data while using CBC mode, which is the default for this class.

@samrueby
Added link in AesManaged to article warning of timing vulnerabilities…
1867846 
… with CBC-mode symmetric decryption using padding.
@mairaw mairaw requested a review from bartonjs June 17, 2019 18:23
@mairaw
Copy link
Contributor

mairaw commented Jun 17, 2019

Thanks for your contribution @samrueby. @bartonjs can you please take a look? I can make the suggestions from a content perspective after the tech review is done.

@bartonjs
Copy link
Member

Seems fine to me.

mairaw
mairaw previously requested changes Jun 19, 2019
View reviewed changes
Copy link
Contributor

@mairaw mairaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @samrueby. Just have a small edit request before merging this.

@mairaw
Copy link
Contributor

mairaw commented Jun 19, 2019

It would probably be good to add this info to the Mode property remarks as well.

@samrueby
Copy link
Contributor Author

Possibly- but according to the article a lot of other types are apparently affected too:

Aes
AesCng
AesCryptoServiceProvider
AesManaged
DES
DESCryptoServiceProvider
RC2
RC2CryptoServiceProvider
Rijndael
RijndaelManaged
TripleDES
TripleDESCng
TripleDESCryptoServiceProvider

Should all of them be updated?

@mairaw
Copy link
Contributor

mairaw commented Aug 6, 2019

@bartonjs can you comment on that?

@mairaw mairaw added this to the June 2019 milestone Aug 6, 2019
@mairaw mairaw added the waiting-on-feedback Indicates PRs that are waiting for feedback from SMEs before they can be merged label Aug 6, 2019
@bartonjs
Copy link
Member

bartonjs commented Aug 7, 2019

Yes, all of those named types are similarly affected (by having CBC+PKCS7 be the default and CBC with any removable padding being what has the vulnerabilty).

@tdykstra
Copy link
Contributor

@samrueby Do you want to update all of the other types you listed, or should we merge this PR and create an issue to track the work of updating the others?

@tdykstra tdykstra added needs-author-action An issue or pull request that requires more info or actions from the author. and removed waiting-on-feedback Indicates PRs that are waiting for feedback from SMEs before they can be merged labels Oct 25, 2019
@BillWagner BillWagner modified the milestones: June 2019, March 2020 Mar 2, 2020
@BillWagner
Copy link
Member

@tdykstra

At this point, I think we should merge this, and create a new issue for the remaining changes. Do you agree?

@BillWagner BillWagner self-assigned this Mar 2, 2020
@BillWagner BillWagner dismissed mairaw’s stale review March 2, 2020 21:02

Tom and I are picking up these PRs

@BillWagner BillWagner merged commit 7ba3f11 into dotnet:master Mar 2, 2020
@BillWagner BillWagner mentioned this pull request Mar 2, 2020
Open
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tdykstra tdykstra tdykstra approved these changes

@mairaw mairaw mairaw left review comments

@bartonjs bartonjs Awaiting requested review from bartonjs

Assignees

@BillWagner BillWagner

@tdykstra tdykstra

@samrueby samrueby

Labels
needs-author-action An issue or pull request that requires more info or actions from the author.
Projects
None yet
Milestone
March 2020
Development

Successfully merging this pull request may close these issues.
5 participants
@samrueby @mairaw @bartonjs @tdykstra @BillWagner